feat: migrate chat delete flow to dedicated api#1612
Merged
sweetmantech merged 3 commits intotestfrom Mar 31, 2026
Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughLocal room-delete route removed; client DeleteConfirmationModal now uses a new useDeleteChat hook which obtains a Bearer access token and calls an external DELETE /api/chats endpoint via lib/deleteChat instead of the previous local POST route. Changes
Sequence Diagram(s)sequenceDiagram
participant Modal as DeleteConfirmationModal
participant Hook as useDeleteChat
participant Auth as usePrivy (AccessTokenProvider)
participant API as External API (/api/chats)
Modal->>Hook: deleteChat(roomId)
Hook->>Auth: getAccessToken()
Auth-->>Hook: Bearer <token> or null
alt token present
Hook->>API: DELETE /api/chats { id: roomId } + Authorization: Bearer <token>
API-->>Hook: 200 OK / error payload
alt response OK
Hook-->>Modal: resolves (void)
else response error
Hook-->>Modal: throws Error(result.error or fallback)
end
else no token
Hook-->>Modal: throws Error("Not authenticated")
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/Sidebar/Modals/DeleteConfirmationModal.tsx`:
- Around line 27-28: The code uses apiOverride (from useApiOverride) directly as
baseUrl (falling back to NEW_API_BASE_URL) and may send the bearer token to
attacker-controlled origins; update DeleteConfirmationModal to validate and
sanitize apiOverride before using it for authenticated requests: allow only
same-origin or a small whitelist of trusted origins (or require a relative
path), reject or ignore overrides that parse to an external origin, and ensure
when an override is rejected you fall back to NEW_API_BASE_URL; apply the same
validation wherever baseUrl is computed from useApiOverride (e.g., the other
baseUrl usages referenced).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cee8c176-93d6-4dfe-bb52-b61884a27536
📒 Files selected for processing (2)
app/api/room/delete/route.tscomponents/Sidebar/Modals/DeleteConfirmationModal.tsx
💤 Files with no reviewable changes (1)
- app/api/room/delete/route.ts
| @@ -48,6 +54,11 @@ const DeleteConfirmationModal = ({ isOpen, onClose, chatRoom, chatRooms, onDelet | |||
| : 'Delete'; | |||
|
|
|||
| const handleDelete = async () => { | |||
Collaborator
There was a problem hiding this comment.
SRP - Move this handleDelete function with accessToken to a standalone hook file.
- Move API call logic (auth, fetch, error handling) into hooks/useDeleteChat.ts - DeleteConfirmationModal now delegates to the hook - Remove dead result.message check (API returns result.error) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
hooks/useDeleteChat.ts
Outdated
| * Returns a hook with a deleteChat function. | ||
| */ | ||
| export function useDeleteChat() { | ||
| const accessToken = useAccessToken(); |
Collaborator
There was a problem hiding this comment.
Replace this to use the getAccessToken call directly from the Privy SDK to follow KISS principle.
hooks/useDeleteChat.ts
Outdated
| const apiOverride = useApiOverride(); | ||
| const baseUrl = apiOverride || NEW_API_BASE_URL; | ||
|
|
||
| const deleteChat = async (roomId: string): Promise<void> => { |
Collaborator
There was a problem hiding this comment.
Would this function be more efficient written as a useMutate from tanstack?
- Use getAccessToken from Privy SDK instead of useAccessToken hook (KISS) - Wrap deleteChat in useMutation for proper pending/error state - Extract API call to lib/chats/deleteChat.ts (SRP) - Remove manual isDeleting state — useMutation.isPending handles it Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
hooks/useDeleteChat.ts (1)
12-23: Restrict API override to trusted origins in production.
apiOverride(controlled via?api=<url>query parameter) is used in bothuseDeleteChatanduseChatTransportto override the API base URL. Both hooks attach bearer tokens to requests sent to the overridden URL without validating the domain. The current URL validation inuseApiOverrideonly checks syntax (new URL()) and doesn't enforce a domain whitelist.To prevent token exfiltration, either:
- Disable API override in production (check
IS_PRODfromlib/consts.ts)- Implement a whitelist of allowed domains before accepting the override
- Remove query parameter control and limit overrides to environment variables only
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/useDeleteChat.ts` around lines 12 - 23, The apiOverride passed into useDeleteChat (and likewise used by useChatTransport) can cause bearer tokens to be sent to arbitrary URLs; update the logic that derives baseUrl (which currently uses apiOverride || NEW_API_BASE_URL) to reject or ignore untrusted overrides by checking IS_PROD from lib/consts.ts and enforcing a domain whitelist (or disabling overrides entirely in production): modify useApiOverride (and callers useDeleteChat/useChatTransport) to validate the override against an allowlist of trusted origins before using apiOverride, or when IS_PROD is true always use NEW_API_BASE_URL and never accept query-based overrides.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@hooks/useDeleteChat.ts`:
- Around line 28-32: The code in useDeleteChat.ts calls response.json()
unconditionally which will throw for 204/empty or non-JSON DELETE responses;
change the flow in the delete handler to check response.ok (or Content-Type /
Content-Length) before parsing: if (!response.ok) read and parse the body (if
present) to include error details, otherwise skip parsing for 204 and treat as
success—use the same approach as lib/messages/clientDeleteTrailingMessages.ts
and adjust the block around response and result to conditionally parse or
short-circuit on response.ok/empty body.
---
Nitpick comments:
In `@hooks/useDeleteChat.ts`:
- Around line 12-23: The apiOverride passed into useDeleteChat (and likewise
used by useChatTransport) can cause bearer tokens to be sent to arbitrary URLs;
update the logic that derives baseUrl (which currently uses apiOverride ||
NEW_API_BASE_URL) to reject or ignore untrusted overrides by checking IS_PROD
from lib/consts.ts and enforcing a domain whitelist (or disabling overrides
entirely in production): modify useApiOverride (and callers
useDeleteChat/useChatTransport) to validate the override against an allowlist of
trusted origins before using apiOverride, or when IS_PROD is true always use
NEW_API_BASE_URL and never accept query-based overrides.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fb2a264d-6d94-4be7-ac4c-b939ee291fc9
📒 Files selected for processing (2)
components/Sidebar/Modals/DeleteConfirmationModal.tsxhooks/useDeleteChat.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- components/Sidebar/Modals/DeleteConfirmationModal.tsx
sweetmantech
approved these changes
Mar 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
POST /api/room/deleteto dedicatedDELETE /api/chatsuseAccessTokenuseApiOverrideapp/api/room/delete/route.tsValidation
pnpm exec eslint components/Sidebar/Modals/DeleteConfirmationModal.tsxSummary by CodeRabbit
Refactor
New Features
Bug Fixes